Repository navigation
Sponsor Management UI - #2972
JacobCoffee wants to merge 123 commits into
Conversation
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
There was a problem hiding this comment.
Pull request overview
This PR introduces a staff-facing Sponsor Management UI under the sponsors app, plus supporting infrastructure for email testing and outbound-notification auditing/logging.
Changes:
- Add a new
/sponsors/manage/management UI (templates, forms, URLs) for handling sponsorships, packages, benefits, contacts, contracts, and notifications. - Persist outbound sponsorship notification sends to a new
SponsorshipNotificationLogmodel and display history in the management UI. - Improve local dev ergonomics by adding MailDev + SMTP-backed local email configuration; update sponsor contract email templates; bump
django-storages.
Reviewed changes
Copilot reviewed 43 out of 44 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
uv.lock |
Bumps django-storages from 1.14.4 to 1.14.6 (lockfile update). |
pydotorg/settings/local.py |
Uses SMTP backend when EMAIL_HOST is set (for MailDev), otherwise console backend. |
docker-compose.yml |
Adds a maildev service and wires EMAIL_HOST/EMAIL_PORT into the web service. |
apps/sponsors/use_cases.py |
Logs each sent sponsorship notification to SponsorshipNotificationLog. |
apps/sponsors/urls.py |
Mounts the management UI under sponsors/manage/. |
apps/sponsors/templates/sponsors/manage/sponsorship_notify.html |
UI to preview/send a notification for a single sponsorship. |
apps/sponsors/templates/sponsors/manage/sponsorship_list.html |
Sponsorship list UI with filters, bulk actions, and live search. |
apps/sponsors/templates/sponsors/manage/sponsorship_edit.html |
Edit sponsorship details (package/fee/year). |
apps/sponsors/templates/sponsors/manage/sponsorship_detail.html |
Sponsorship detail/review UI incl. contracts, benefits, assets, communications. |
apps/sponsors/templates/sponsors/manage/sponsorship_approve_signed.html |
Approve sponsorship while uploading a signed contract document. |
apps/sponsors/templates/sponsors/manage/sponsorship_approve.html |
Approve sponsorship (creates draft contract) with quick date helpers. |
apps/sponsors/templates/sponsors/manage/sponsor_edit.html |
Create/edit sponsor company data. |
apps/sponsors/templates/sponsors/manage/package_list.html |
Package list UI grouped by year. |
apps/sponsors/templates/sponsors/manage/package_form.html |
Create/edit a sponsorship package. |
apps/sponsors/templates/sponsors/manage/package_confirm_delete.html |
Confirm-delete UI for packages. |
apps/sponsors/templates/sponsors/manage/notification_template_list.html |
List notification templates in the management UI. |
apps/sponsors/templates/sponsors/manage/notification_template_form.html |
Create/edit notification templates with variable copy helpers. |
apps/sponsors/templates/sponsors/manage/notification_template_confirm_delete.html |
Confirm-delete UI for notification templates. |
apps/sponsors/templates/sponsors/manage/notification_history.html |
Management UI listing sent notifications (log history). |
apps/sponsors/templates/sponsors/manage/guide.html |
End-user guide for PSF sponsorship team workflows. |
apps/sponsors/templates/sponsors/manage/dashboard.html |
Management dashboard summary for a selected year. |
apps/sponsors/templates/sponsors/manage/current_year_form.html |
UI to set the active sponsorship year. |
apps/sponsors/templates/sponsors/manage/contract_send.html |
UI for generating/sending contracts and internal review. |
apps/sponsors/templates/sponsors/manage/contract_execute.html |
UI to upload signed contract and execute/finalize sponsorship. |
apps/sponsors/templates/sponsors/manage/contact_form.html |
Create/edit sponsor contact roles and info. |
apps/sponsors/templates/sponsors/manage/clone_year.html |
Clone packages/benefits from one year to another. |
apps/sponsors/templates/sponsors/manage/bulk_notify.html |
UI to send notifications in bulk across sponsorships. |
apps/sponsors/templates/sponsors/manage/benefit_list.html |
Benefit list UI with filtering/pagination. |
apps/sponsors/templates/sponsors/manage/benefit_form.html |
Create/edit benefit + manage feature configurations and related data. |
apps/sponsors/templates/sponsors/manage/benefit_confirm_delete.html |
Confirm-delete UI for benefits. |
apps/sponsors/templates/sponsors/manage/benefit_config_form.html |
Create/edit benefit feature configuration objects. |
apps/sponsors/templates/sponsors/manage/_base.html |
Management UI shell + global JS table-sorting helper. |
apps/sponsors/templates/sponsors/email/sponsor_contract_subject.txt |
Replaces placeholder subject with a real contract subject template. |
apps/sponsors/templates/sponsors/email/sponsor_contract.txt |
Replaces placeholder contract email body with real copy. |
apps/sponsors/models/notifications.py |
Adds SponsorshipNotificationLog model to persist sent notifications. |
apps/sponsors/models/__init__.py |
Re-exports SponsorshipNotificationLog. |
apps/sponsors/migrations/0104_add_notification_log.py |
Migration creating the notification log table. |
apps/sponsors/management/commands/seed_sponsor_manage_data.py |
Dev-only command to seed realistic data for the management UI. |
apps/sponsors/manage/urls.py |
URL routing for the management UI. |
apps/sponsors/manage/forms.py |
Forms backing the management UI (benefits/packages/sponsorship ops/notifications/etc.). |
apps/sponsors/manage/__init__.py |
Documents that the manage UI is staff/group restricted. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 43 out of 44 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 43 out of 43 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 43 out of 43 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 49 out of 51 changed files in this pull request and generated 5 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
may be best to split on 6c06c38 (before all the ai fun bits), merge that, and pull anything after into other PRs |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 49 out of 51 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
🔵 Needs a closer look
The notification history template currently renders an incorrect count badge due to applying |length to paginator.count (an integer).
Review details
Suppressed comments (1)
apps/sponsors/templates/sponsors/manage/notification_history.html:24
paginator.countis an integer, so piping it through|lengthalways yields 0 (TypeError -> 0). This makes the “Sent Notifications” badge incorrect whenever pagination is enabled. Use the count directly (orlogs|lengthonly when there is no paginator).
- Files reviewed: 49/51 changed files
- Comments generated: 0 new
- Review effort level: Lite
# Conflicts: # apps/users/views.py # docker-compose.yml # pydotorg/settings/local.py
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
|
|
||
| Contracts are emailed to verified sponsor contacts. Website downloads are limited | ||
| to authorized sponsorship managers and contract administrators, including signed | ||
| copies. New contract files use private storage; public media is unchanged. |
There was a problem hiding this comment.
🔴 After deploying by these docs, staff can no longer open or re-send contracts created before the upgrade, and the old files stay on public media. The docs say "public media is unchanged" and never mention remediate_contract_storage. Migration 0107 points every existing Contract file field at private storage but moves no files. Fix: document ./manage.py remediate_contract_storage --apply (then --delete-legacy) as a required deploy step, run right after migrate. Do the same for remediate_sponsor_asset_storage, and say that legacy links return 404 until each command has run.
Why this was flagged
Migration 0107 (apps/sponsors/migrations/0107_contract_private_storage_and_original_sponsorship.py) only runs AlterField to storage=get_contract_storage and backfills original_sponsorship. No file is copied. In production S3ContractStorage uses the contracts-private key prefix (pydotorg/settings/cabotage.py), but legacy objects live under MediaStorage's media/ prefix, and nothing falls back to default_storage except custom_storages/remediation.py. So for every pre-upgrade contract, download_contract_document reaches _private_attachment (apps/sponsors/views.py), whose field_file.open raises FileNotFoundError, giving a 404. Re-sending an Awaiting Signature contract fails as well. On the base branch these files were served from media. The one thing that fixes this is remediate_contract_storage --apply, and docs/source/administration.rst:286-288 never names it. Those lines instead say "public media is unchanged".
Verification: Migration 0107 only does AlterField on document, document_docx and signed_document, plus RunPython(backfill_original_sponsorship). In cabotage, SPONSORS_CONTRACT_STORAGE_LOCATION = "contracts-private" (pydotorg/settings/cabotage.py:54-55). The Procfile has release: python manage.py migrate --noinput. remediate_contract_storage appears nowhere in the docs, Makefile or Procfile.
| if clone_packages: | ||
| pkgs = [p.clone(year)[0] for p in self.packages.all()] | ||
| new_benefit.packages.add(*pkgs) | ||
| clauses = [lc.clone() for lc in self.legal_clauses.all()] | ||
| new_benefit.legal_clauses.add(*clauses) | ||
| for cfg in self.features_config.all(): |
There was a problem hiding this comment.
🔴 Staff who clone a year with "Clone packages (tiers)" unchecked still get packages created in the target year, and the page reports "Cloned 0 package(s)". SponsorshipBenefit.clone(clone_packages=False) skips only self.packages. It still runs cfg.clone(new_benefit) for every feature configuration. TieredBenefitConfiguration.get_clone_kwargs (benefits.py:485) calls self.package.clone(year=...), which get-or-creates the package in the target year. Fix: when clone_packages is False, never create packages. Point each tiered config at the target year's existing package with the same slug, or skip it when there is none.
Why this was flagged
Staff open /sponsors/manage clone-year, untick "Clone packages (tiers)" and keep "Clone benefits" ticked. CloneYearView.form_valid (apps/sponsors/manage/views.py:942-946) then calls benefit.clone(target_year, clone_packages=False). In SponsorshipBenefit.clone (apps/sponsors/models/sponsorship.py:599-605), the flag guards only the self.packages copy. The loop for cfg in self.features_config.all(): cfg.clone(new_benefit) still runs. For a TieredBenefitConfiguration, get_clone_kwargs (apps/sponsors/models/benefits.py:482-486) runs self.package.clone(year=new_benefit.year). That is SponsorshipPackage.objects.get_or_create(slug=..., year=...) (sponsorship.py:111-120). So the target year still gets every package that any tiered benefit uses. Meanwhile cloned_packages stays 0, so the success message says "Cloned 0 package(s)". No test covers clone_packages=False: apps/sponsors/manage/tests.py only posts True.
Verification: In apps/sponsors/models/sponsorship.py:598-605, if clone_packages: guards only self.packages (lines 599-601). For a TieredBenefitConfiguration, benefits.py:482-486 runs kwargs["package"], _ = self.package.clone(year=new_benefit.year). The clone_packages option is new in this diff and does not do what it says for tiered benefits.
No description provided.